Skip to content

[OBJECT] Make object info macro compatible with CRTP#636

Merged
tqchen merged 1 commit into
apache:mainfrom
tqchen:tvm-ffi-support-crtp-object-info-declarations-after-runtimetypeindex-cache
Jun 19, 2026
Merged

[OBJECT] Make object info macro compatible with CRTP#636
tqchen merged 1 commit into
apache:mainfrom
tqchen:tvm-ffi-support-crtp-object-info-declarations-after-runtimetypeindex-cache

Conversation

@tqchen

@tqchen tqchen commented Jun 18, 2026

Copy link
Copy Markdown
Member

This PR makes TVM_FFI_DECLARE_OBJECT_INFO_PREDEFINED_TYPE_KEY compatible with CRTP-style object declarations, and fixes the MSVC build failure seen with that pattern.

When the macro is expanded inside a CRTP base template, the cached _type_index member is declared in the current CRTP base specialization. The _type_index initializer now calls _GetOrAllocRuntimeTypeIndex() through current-class lookup instead of qualifying that function through the CRTP leaf type, which avoids MSVC lookup failures on incomplete CRTP leaf types.

Other TypeName:: lookups are preserved so leaf-provided metadata such as _type_key and child-slot overrides continue to be used.

A generic CRTPObject<T> / LeafObject regression test is added in test_object.cc.

Validation:

  • cmake -S . -B build-crtp -G Ninja -DCMAKE_BUILD_TYPE=Release -DTVM_FFI_USE_EXTRA_CXX_API=ON -DTVM_FFI_BUILD_TESTS=ON
  • ninja -C build-crtp tvm_ffi_tests
  • ./build-crtp/lib/tvm_ffi_tests --gtest_filter=Object.CRTPObjectInfo:Object.TypeInfo
  • ./build-crtp/lib/tvm_ffi_tests --gtest_filter=Object.*
  • pre-commit run --files include/tvm/ffi/object.h tests/cpp/test_object.cc

@tqchen tqchen force-pushed the tvm-ffi-support-crtp-object-info-declarations-after-runtimetypeindex-cache branch 2 times, most recently from db00642 to db1adc2 Compare June 18, 2026 22:33
@tqchen tqchen changed the title [OBJECT] Fix CRTP object info type index lookup [OBJECT] Make object info macro compatible with CRTP Jun 18, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the TVM_FFI_DECLARE_OBJECT_INFO_PREDEFINED_TYPE_KEY macro to use unqualified lookups for several type properties and adds a test case for CRTP (Curiously Recurring Template Pattern) objects. Feedback indicates that removing the TypeName:: prefix from _type_child_slots and _type_child_slots_can_overflow breaks the ability of CRTP leaf classes to override these constants, as unqualified lookup in the base template will resolve to the base class's values. It is recommended to revert these two parameters to use qualified TypeName:: lookup.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread include/tvm/ffi/object.h Outdated
@tqchen tqchen force-pushed the tvm-ffi-support-crtp-object-info-declarations-after-runtimetypeindex-cache branch from db1adc2 to a0cabb6 Compare June 18, 2026 22:37
@tqchen tqchen force-pushed the tvm-ffi-support-crtp-object-info-declarations-after-runtimetypeindex-cache branch from a0cabb6 to 8bfbb8f Compare June 18, 2026 22:48
@tqchen tqchen merged commit 102fe0e into apache:main Jun 19, 2026
9 checks passed
tqchen added a commit to apache/tvm that referenced this pull request Jun 19, 2026
This PR bumps the vendored `3rdparty/tvm-ffi` submodule to the latest
upstream commit after the RuntimeTypeIndex optimization and CRTP
compatibility follow-up.

Included tvm-ffi updates:
- apache/tvm-ffi#633: Phase out Python 3.8 wheel builds
- apache/tvm-ffi#635: Ignore existing DLLs in Windows wheel repair
- apache/tvm-ffi#634: Optimize dynamic RuntimeTypeIndex loads
- apache/tvm-ffi#636: Make object info macro compatible with CRTP

Validation:
- Built `tvm_runtime` and `tvm_compiler` in a Release CMake build.
- `git diff --check` passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants